Issue #1292 mete grids from imod5#1293
Conversation
JoerivanEngelen
changed the base branch from
master
to
issue_#1260_from_imod5_data_metaswap
…mete_grids_from_imod5
|
|
||
| def find_in_file_list(filename: str, paths: list[str]) -> str: | ||
| for file in paths: | ||
| if filename == Path(file[0]).name.lower(): |
There was a problem hiding this comment.
What does the zeroth index of a file return?
There was a problem hiding this comment.
To be frank, I copy+pasted that from the prototype drafted by @HendrikKok. It's worthy of an investigation, because it serves some explanation and might even point to something clumsy in what is done in open_projectfile_data.
There was a problem hiding this comment.
This is a relict of how read_projectfile parses the data: A list for each line in the file, split into elements for each comma in the line. However, in the "EXTRA FILES" text block, there is only one element per line, causing the unnecessary nested list. I'm not sure if I want complicate the logic in read_projectfile with edge cases, but I could concatenate the lists in open_projectfile_data as there all data processing is done.
| mete_grid_path = Path(mete_grid_path) | ||
|
|
||
| f = open(mete_grid_path, "r") | ||
| lines = f.readlines() |
There was a problem hiding this comment.
Instead of readlines you can use readline to get the first line
Furthermore i wiould suggest using a context to make sure the file is closed when you're done reading it
with open(mete_grid_path, 'r') as file:
first_line = file.readline()
meteo_filepath = Path(first_line.split(",")[column_nr].replace('"', ""))
There was a problem hiding this comment.
Oomph good one! Missed the context when opening the file 😱
| assert_equal(results["svat"], np.array([1, 2, 3, 4, 5, 6])) | ||
| assert_equal(results["row"], np.array([1, 2, 1, 2, 2, 2])) | ||
| assert_equal(results["column"], np.array([2, 2, 2, 2, 2, 1])) | ||
| assert_equal(results["column"], np.array([2, 2, 2, 2, 2, 3])) |
There was a problem hiding this comment.
Was there a mistake in the old test?
There was a problem hiding this comment.
I guess because you moved the coordinates in the tests quite a bit it seems?
There was a problem hiding this comment.
Yeah, the original test fixtures had increasing y-coords, whereas most or all usecases are decreasing. This updates that.
| mete_grid_path = Path(mete_grid_path) | ||
|
|
||
| with open(mete_grid_path, "r") as f: | ||
| first_line = f.readline() |
There was a problem hiding this comment.
MetaSWAP also supports constants as input (I found out recently). According to the manual there should be at least one meteo-file present. I would suggest a open_first_present_meteo_grid method.
There was a problem hiding this comment.
Implemented!
|
|
3 participants
Fixes #1292
Description
Context: MetaSWAP can map meteorological grids, which have a coarser grid but a very fine time resolution, to its svats.
The
mete_grid.inpfile contains paths to meteorological information, stored in ASCII grids, and there are separate mapping files"svat2precgrid.inp"and"svat2etrefgrid.inp". Nota bene: MetaSWAP has hardcoded filenames for packages, hence why I can search for "mete_grid.inp".Adds the following:
MeteoGridCopyclass to copy "mete_grid.inp" files, to avoid having to read and write the crazy amount of meteo files that exist in existing databases.MeteoMapping.from_imod5_datamethod which looks for the first ascii grid in themete_grid.inpfile, and reads that to derive mappings from meteorological cell to metaswap grid cell.Imod5DataDicttype alias, which can be implemented for otherfrom_imod5_datamethods in the future.Only thing I doubt now is that I naively copy
mete_grid.inp, which works when the paths to ASCII grids are absolute, but not when they are relative. I don't think iMOD5 does this as well, so present databases all contain absolute paths. @HendrikKok do you foresee a usecase to support relative paths inMeteoGridCopy?Checklist
Issue #nr, e.g.Issue #737